Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor nrn_mlh_gsort #2692

Merged
merged 5 commits into from
Jan 29, 2024
Merged

Refactor nrn_mlh_gsort #2692

merged 5 commits into from
Jan 29, 2024

Conversation

JCGoran
Copy link
Contributor

@JCGoran JCGoran commented Jan 29, 2024

JCGoran and others added 3 commits January 24, 2024 14:29
@JCGoran
Refactor nrn_mlh_gsort
fe9ef10 
* use `std::sort` instead of a custom one
* remove unnecessary macros and inline functions
* add tests
* fixes #2681
Format
ad9bc90
@alkino
Merge branch 'master' into jelic/refactor_nrn_sort
316887b
@alkino alkino marked this pull request as ready for review January 29, 2024 09:08
@alkino alkino enabled auto-merge (squash) January 29, 2024 09:34
@alkino alkino disabled auto-merge January 29, 2024 09:38
@codecov Codecov
Copy link

codecov bot commented Jan 29, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (128c684) 66.30% compared to head (2a7de92) 66.35%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2692      +/-   ##
==========================================
+ Coverage   66.30%   66.35%   +0.04%     
==========================================
  Files         558      559       +1     
  Lines      108434   108395      -39     
==========================================
+ Hits        71899    71921      +22     
+ Misses      36535    36474      -61

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@azure-pipelines Azure Pipelines
Copy link

✔️ 4a00c66 -> Azure artifacts URL

@bbpbuildbot

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

NEURON ModelDB CI: launching for 4a00c66 via its drop url

@JCGoran
Copy link
Contributor Author

JCGoran commented Jan 29, 2024

Models using this function (found by grepping):

  • 186768
  • 185858
  • 140881
  • 144586
  • 144538
  • 189154
  • 97868
  • 266851
  • 258738
  • 154096
  • 150245
  • 144549
  • 146949
  • 141505
  • 138379
  • 116838
  • 168874
  • 147141
  • 106891
  • 260015
  • 195615
  • 139421
  • 183014
  • 151282
  • 116862
  • 267511
  • 136095

Verification for those specific models: https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/7694942608

@JCGoran JCGoran requested review from 1uc and alkino January 29, 2024 11:59
nrnhines
nrnhines approved these changes Jan 29, 2024
View reviewed changes
Copy link
Member

@nrnhines nrnhines left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification!

test/unit_tests/iovec.cpp Outdated Show resolved Hide resolved
1uc
1uc reviewed Jan 29, 2024
View reviewed changes
src/ivoc/ivocvect.cpp Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

NEURON ModelDB CI: 4a00c66 -> download reports from here

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jan 29, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

8 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #189119 (:no_entry:) have been uploaded here!

Status and direct links:

@azure-pipelines Azure Pipelines
Copy link

✔️ 2a7de92 -> Azure artifacts URL

@alkino alkino merged commit ed0b461 into master Jan 29, 2024
37 checks passed
@alkino alkino deleted the jelic/refactor_nrn_sort branch January 29, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1uc 1uc 1uc approved these changes

@nrnhines nrnhines nrnhines approved these changes

@alkino alkino Awaiting requested review from alkino

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rewrite nrn_mlh_gsort with std::sort
5 participants
@JCGoran @bbpbuildbot @alkino @nrnhines @1uc